Skip to content

Conversation

liweicheng00
Copy link
Contributor

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

  • When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)
  • All tests are passing

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Other information:
preview

@Alisonispig
Copy link

Alisonispig commented Nov 18, 2021

I think after adding the preview, you can add functions such as the previous picture and the next picture, and even enhance the interface, such as zooming in and out, rotating left and right, just like elementui.https://element.eleme.cn/#/zh-CN/component/image#da-tu-yu-lan Of course, if you are inconvenient, I can also help after the authors merge

@antoine92190
Copy link
Collaborator

Thanks for the PR!
However, the issue is that developers are loosing the flexibility to implement their own implementation when clicking on the image. We should probably add a prop to disable this behaviour, something like media-modal-preview: true/false

@liweicheng00
Copy link
Contributor Author

I had added prop to disable this behavior and change some variable name for more readable. I am not sure if I need to resolve the conflicts before I commit again.

@liweicheng00
Copy link
Contributor Author

I think after adding the preview, you can add functions such as the previous picture and the next picture, and even enhance the interface, such as zooming in and out, rotating left and right, just like elementui.https://element.eleme.cn/#/zh-CN/component/image#da-tu-yu-lan Of course, if you are inconvenient, I can also help after the authors merge

This feature sounds great and I think it is not complex. I will see if I have time for doing this after author merge.

@antoine92190
Copy link
Collaborator

Yes please resolve the conflicts otherwise I won’t be able to merge this PR

@antoine92190
Copy link
Collaborator

I tested the branch locally, it works fine and the idea is good, but the UI is not very beautiful. I'll improve it a bit before we can merge this.

@antoine92190 antoine92190 merged commit bf5752d into advanced-chat:master Nov 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants